Add better submission failure messaging #27180
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These have been cropping up rather often lately, mostly courtesy of linux users, but not only: ppy#26840 ppy#27008 ppy#26962 so this is a proposal for slightly improved messaging for such cases to hopefully get users on the right track. The original error is still logged to network log, so there's no information loss.
Previously, if a `SubmittingPlayer` instance deemed it okay to proceed with gameplay despite submission failure, it would silently log all errors and proceed, but the score would still not be submitted. This feels a bit anti-user in the cases wherein something is genuinely wrong with either the client or web, so things like token verification failures or API failures are now shown as notifications to give the user an indication that something went wrong at all. Selected cases (non-user-playable mod, logged out, beatmap is not online) are still logged silently because those are either known and expected, or someone is messing with things.
peppy
reviewed
Feb 18, 2024
| if (string.IsNullOrEmpty(exception.Message)) | ||
| Logger.Error(exception, "Failed to retrieve a score submission token."); | ||
| Logger.Error(exception, $"{whatWillHappen} Failed to retrieve a score submission token."); |
There was a problem hiding this comment.
diff --git a/osu.Game/Screens/Play/SubmittingPlayer.cs b/osu.Game/Screens/Play/SubmittingPlayer.cs
index 0873f60791..ecb507f382 100644
--- a/osu.Game/Screens/Play/SubmittingPlayer.cs
+++ b/osu.Game/Screens/Play/SubmittingPlayer.cs
@@ -137,11 +137,11 @@ void handleTokenFailure(Exception exception, bool displayNotification = false)
if (displayNotification || shouldExit)
{
string whatWillHappen = shouldExit
- ? "You are not able to submit a score."
- : "The following score will not be submitted.";
+ ? "Play in this state is not permitted."
+ : "Your score will not be submitted.";
if (string.IsNullOrEmpty(exception.Message))
- Logger.Error(exception, $"{whatWillHappen} Failed to retrieve a score submission token.");
+ Logger.Error(exception, $"Failed to retrieve a score submission token.\n\n{whatWillHappen}");
else
{
switch (exception.Message)
@@ -149,11 +149,11 @@ void handleTokenFailure(Exception exception, bool displayNotification = false)
case @"missing token header":
case @"invalid client hash":
case @"invalid verification hash":
- Logger.Log($"{whatWillHappen} Please ensure that you are using the latest version of the official game releases.", level: LogLevel.Important);
+ Logger.Log($"Please ensure that you are using the latest version of the official game releases.\n\n{whatWillHappen}", level: LogLevel.Important);
break;
case @"expired token":
- Logger.Log($"{whatWillHappen} Your system clock is set incorrectly. Please check your system time, date and timezone.", level: LogLevel.Important);
+ Logger.Log($"Your system clock is set incorrectly. Please check your system time, date and timezone.\n\n{whatWillHappen}", level: LogLevel.Important);
break;
default:
Something like this may read better, I think.
Ultimately designing a custom notification for this is probably warranted, at some point.
There was a problem hiding this comment.
Have applied verbatim.
Ultimately designing a custom notification for this is probably warranted, at some point.
Not sure what this would entail.
There was a problem hiding this comment.
Mostly just adding an icon and what not.
peppy
approved these changes
Feb 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Subjective, so I accept the possibility of this getting dumpstered on sight.
Use better messaging for selected submission failure reasons
Some failure reasons related to more stringent integrity checks have been cropping up rather often lately, mostly courtesy of linux users, but not only:
so this is a proposal for slightly improved messaging for such cases to hopefully get users on the right track.
The original error is still logged to network log, so there's no information loss.
Show selected submission failure messages even in solo
Previously, if a
SubmittingPlayerinstance deemed it okay to proceed with gameplay despite submission failure, it would silently log all errors and proceed, but the score would still not be submitted. This feels a bit anti-user in the cases wherein something is genuinely wrong with either the client or web, so things like token verification failures or API failures are now shown as notifications to give the user an indication that something went wrong at all.Selected cases (non-user-playable mod, logged out, beatmap is not online) are still logged silently because those are either known and expected, or someone is messing with things.